Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Task 42: structure validator CLI output #47

Closed
wants to merge 4 commits into from
Closed

Task 42: structure validator CLI output #47

wants to merge 4 commits into from

Conversation

nargis-sultani
Copy link
Contributor

No description provided.

@nargis-sultani
Copy link
Contributor Author

Fixing linter issues

@github-actions
Copy link

github-actions bot commented Sep 14, 2023 •
edited
Loading

Coverage report

The coverage rate went from 81.08% to 78.4% ⬇️
The branch rate is 57%.

0% of new lines are covered.

Diff Coverage details (click to unfold)

src/validator/create_schemas.py

0% of new lines are covered (0% of the complete file).
Missing lines: 4, 5, 33, 45, 46, 47, 48, 49, 50, 51, 52, 53, 55, 57, 58, 60, 61, 64, 69, 71, 72, 75, 77, 79, 85, 89, 90, 91, 92, 93, 94, 96, 107, 109, 112, 113, 114, 115, 117, 118, 119, 121

src/validator/main.py

0% of new lines are covered (0% of the complete file).
Missing lines: 11, 30

@aharjati
Copy link
Contributor

aharjati commented Sep 14, 2023 •
edited
Loading

Is this ready for review?

just heads up: I see this error when running validator against the good and bad CSV files:

le "/workspaces/regtech-data-validator/src/validator/main.py", line 11, in <module>
    from create_schemas import validate_phases_by_lei
ImportError: cannot import name 'validate_phases_by_lei' from 'create_schemas' (/workspaces/regtech-data-validator/src/validator/create_schemas.py)
/workspaces/regtech-dat

Also, we should add unit tests for create_schemas. Create_schemas was being used for local testing so that's why we didn't have unit tests. Now, we are using it to generate JSON data that will be called by API so we should add unit tests for it. I created unit tests for create_schemas in my WIP branch: src/tests/test_schema_functions.py.
so maybe we can reuse some of the unit tests

@hkeeler hkeeler mentioned this pull request Sep 14, 2023
Merged
@nargis-sultani
Copy link
Contributor Author

Is this ready for review?

just heads up: I see this error when running validator against the good and bad CSV files:

le "/workspaces/regtech-data-validator/src/validator/main.py", line 11, in <module>
    from create_schemas import validate_phases_by_lei
ImportError: cannot import name 'validate_phases_by_lei' from 'create_schemas' (/workspaces/regtech-data-validator/src/validator/create_schemas.py)
/workspaces/regtech-dat

Also, we should add unit tests for create_schemas. Create_schemas was being used for local testing so that's why we didn't have unit tests. Now, we are using it to generate JSON data that will be called by API so we should add unit tests for it. I created unit tests for create_schemas in my WIP branch: src/tests/test_schema_functions.py. so maybe we can reuse some of the unit tests

I don't see that locally, working fine. But I can add the unit tests.

aharjati
aharjati suggested changes Sep 15, 2023
View reviewed changes
src/validator/main.py
phase_2_sblar_schema(df, lazy=True)
except SchemaErrors as errors:
print_schema_errors(errors, "Phase 2")
print(validate_phases_by_lei(df, lei))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this validate_phases_by_lei does not exist in create_schemas.
If you run /usr/local/bin/python /workspaces/regtech-data-validator/src/validator/main.py SBL_Validations_SampleData_GoodFile_03312 023.csv , you'll see this error ImportError: cannot import name 'validate_phases_by_lei' from 'create_schemas' (/workspaces/regtech-data-validator/src/validator/create_schemas.py)

src/validator/create_schemas.py
def get_phase_1_schema_for_lei(lei: str = None):
return get_schema_by_phase_for_lei(phase_1_template, "phase_1", lei)


def get_phase_2_schema_for_lei(lei: str = None):
return get_schema_by_phase_for_lei(phase_2_template, "phase_2", lei)


def validate(schema: DataFrameSchema, df: pd.DataFrame):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: functions in #32 does not handle LEI so you'll need update them to handle LEI check

src/validator/create_schemas.py Outdated
return findings


def validate_phases(phase1: DataFrameSchema, phase2: DataFrameSchema, df: pd.DataFrame) -> list:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: functions in #32 does not handle LEI so you'll need update them to handle LEI check

@nargis-sultani nargis-sultani deleted the features/42_structure_validator_cli_output branch September 15, 2023 16:34
@nargis-sultani
Copy link
Contributor Author

closing it because it works on my local branch but not on this PR, will recreate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aharjati aharjati aharjati requested changes

@hkeeler hkeeler Awaiting requested review from hkeeler

@lchen-2101 lchen-2101 Awaiting requested review from lchen-2101

@guffee23 guffee23 Awaiting requested review from guffee23

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nargis-sultani @aharjati